Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow configuring cssnano #6422

Merged
merged 11 commits into from
Jun 12, 2021
Merged

Conversation

aminya
Copy link
Contributor

@aminya aminya commented Jun 7, 2021

↪️ Pull Request

Fixes #1721

Already fixed. Closes #6102

💻 Examples

🚨 Test instructions

Install cssnano-preset-advanced, then make the following file at the project root.

cssnano.config.js

module.exports = {
  preset: [require("cssnano-preset-advanced")]
}

Build and pack this cssnano package, then install it in your project. Using pnpm you can simply do this in your package.json

  "pnpm": {
    "overrides": {
      "@parcel/optimizer-cssnano": "link:../parcel/packages/optimizers/cssnano"
    }
  },

✔️ PR Todo

  • Added/updated unit tests for this change
  • Filled out test instructions (In case there aren't any unit tests)
  • Included links to related issues/PRs

@height
Copy link

height bot commented Jun 7, 2021

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@aminya aminya force-pushed the cssnano-configuration branch 2 times, most recently from 07160d3 to 20b9d3c Compare June 7, 2021 11:11
@devongovett
Copy link
Member

Thanks. FWIW, after #6379 lands, this will need to be refactored to move the config loading into the plugin's loadConfig method, which will handle properly invalidating the cache when the config changes.

@aminya
Copy link
Contributor Author

aminya commented Jun 10, 2021

Thanks. FWIW, after #6379 lands, this will need to be refactored to move the config loading into the plugin's loadConfig method, which will handle properly invalidating the cache when the config changes.

Do you mean this?

diff --git a/packages/optimizers/cssnano/src/CSSNanoOptimizer.js b/packages/optimizers/cssnano/src/CSSNanoOptimizer.js
index bb96edebb..1ff1a250f 100644
--- a/packages/optimizers/cssnano/src/CSSNanoOptimizer.js
+++ b/packages/optimizers/cssnano/src/CSSNanoOptimizer.js
@@ -9,11 +9,21 @@ import {loadConfig} from '@parcel/utils';
 import path from 'path';

 export default (new Optimizer({
+  async loadConfig({ options }) {
+    return ((await loadConfig(
+      options.inputFS,
+      path.join(options.entryRoot, 'index.css'),
+      ['.cssnanorc ', 'cssnano.config.json', 'cssnano.config.js'],
+      options.projectRoot,
+    )) ?? {}: CSSNanoOptions);
+  }
+
   async optimize({
     bundle,
     contents: prevContents,
     getSourceMapReference,
     map: prevMap,
+    config,
     options,
   }) {
     if (!bundle.env.shouldOptimize) {
@@ -26,14 +36,7 @@ export default (new Optimizer({
       );
     }

-    const userConfig = ((await loadConfig(
-      options.inputFS,
-      path.join(options.entryRoot, 'index.css'),
-      ['.cssnanorc ', 'cssnano.config.json', 'cssnano.config.js'],
-      options.projectRoot,
-    )) ?? {}: CSSNanoOptions);
-
+    const result = await postcss([cssnano(config)]).process(prevContents, {
       // Suppress postcss's warning about a missing `from` property. In this
       // case, the input map contains all of the sources.
       from: undefined,

@mischnic
Copy link
Member

Almost, use config.getConfigFrom instead of utils.loadConfig(): 1ba0bd1#diff-eb084320ed4553866490d232b885f8a522e1f08874f30a75ffb2ca927bafafb9

@aminya
Copy link
Contributor Author

aminya commented Jun 10, 2021

So this one:

❯ git diff
diff --git a/packages/optimizers/cssnano/package.json b/packages/optimizers/cssnano/package.json
index 597fe8cbf..44a02c47c 100644
--- a/packages/optimizers/cssnano/package.json
+++ b/packages/optimizers/cssnano/package.json
@@ -22,7 +22,6 @@
   "dependencies": {
     "@parcel/plugin": "2.0.0-beta.3.1",
     "@parcel/source-map": "2.0.0-rc.4",
-    "@parcel/utils": "2.0.0-beta.3.1",
     "cssnano": "^5.0.5",
     "postcss": "^8.3.0"
   }
diff --git a/packages/optimizers/cssnano/src/CSSNanoOptimizer.js b/packages/optimizers/cssnano/src/CSSNanoOptimizer.js
index bb96edebb..b2ed83fa6 100644
--- a/packages/optimizers/cssnano/src/CSSNanoOptimizer.js
+++ b/packages/optimizers/cssnano/src/CSSNanoOptimizer.js
@@ -5,15 +5,22 @@ import {Optimizer} from '@parcel/plugin';
 import postcss from 'postcss';
 import cssnano from 'cssnano';
 import type {CSSNanoOptions} from 'cssnano'; // TODO the type is based on cssnano 4
-import {loadConfig} from '@parcel/utils';
 import path from 'path';

 export default (new Optimizer({
+  async loadConfig({ config, options }) {
+    return ((await config.getConfigFrom(
+      path.join(options.entryRoot, 'index.css'),
+      ['.cssnanorc', 'cssnano.config.json', 'cssnano.config.js'],
+    )) ?? {}: CSSNanoOptions);
+  },
+
   async optimize({
     bundle,
     contents: prevContents,
     getSourceMapReference,
     map: prevMap,
+    config,
     options,
   }) {
     if (!bundle.env.shouldOptimize) {
@@ -26,14 +33,7 @@ export default (new Optimizer({
       );
     }

-    const userConfig = ((await loadConfig(
-      options.inputFS,
-      path.join(options.entryRoot, 'index.css'),
-      ['.cssnanorc ', 'cssnano.config.json', 'cssnano.config.js'],
-      options.projectRoot,
-    )) ?? {}: CSSNanoOptions);
-
+    const result = await postcss([cssnano(config)]).process(prevContents, {
       // Suppress postcss's warning about a missing `from` property. In this
       // case, the input map contains all of the sources.
       from: undefined,

@mischnic
Copy link
Member

Yes. And you should also remove the space in '.cssnanorc '

@mischnic
Copy link
Member

Actually, you need to do call config.setResult instead of returning the result in loadConfig like here:

config.setResult({

Flow should also already be complaining about that

@aminya
Copy link
Contributor Author

aminya commented Jun 10, 2021

Flow should also already be complaining about that

Well, Flow complains about other things too. config is not an OptimizerOpts property. I am just following based on your comments

@aminya
Copy link
Contributor Author

aminya commented Jun 10, 2021

Flow should also already be complaining about that

Well, Flow complains about other things too. config is not an OptimizerOpts property. I am just following based on your comments

😄 My branch was out of date.

@devongovett devongovett merged commit 23ec831 into parcel-bundler:v2 Jun 12, 2021
@aminya aminya deleted the cssnano-configuration branch July 24, 2021 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade to cssnano 5 in optimizer Parcel not respecting cssnano config
3 participants